Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

This is all done to merge please review this @flash1293 @cchaos.
Will have to do some changes in the examples for that this pr need to be merged. Once Done I will make final changes and changelog entry.

Hey @git-anurag-hub,

Thanks for getting this started! We'll need to make some changes before this is ready to merge.

A couple comments to start:

All changes should be scoped to color_stops files. That is, range_highlight should have no knowledge that a "stepped" option exists. At most, the background prop in range_highlight could be modified to accept multiple background definitions.
So, all stepped background logic should be handled in the same manner and location as the "fixed" and "gradient" options. Aligning the "stepped" gradient creation to the pattern(s) established by "fixed" and "gradient" will be helpful in creating a a background that fits the location of the stop handles.

Also, we'll need to account for custom min-max domains. That is, we cannot assume that the scale will always be from 0-1 or 0-100. Just important to keep in mind for calculations.

Okay will commit changes once I am done with these. 👍
Thanks for help.

@thompsongl It will not be possible to remove the stepped from the range_highlight as making the styles for the fixed or gradient & stepped it totally different but I can do one thing to remove it i.e. I can instead of passing the colors as the prop I can send the background styles directly from the parent component.
Also if in future if range_highlight component is used I will set some default background styles. Is this okay.
If not. Any suggestion from your end are most welcomed. :)

It will not be possible to remove the stepped from the range_highlight as making the styles for the fixed or gradient & stepped it totally different

It is possible to create a stepped linear gradient in the same manner as the other gradient types; see an example here: https://developer.mozilla.org/en-US/docs/Web/CSS/linear-gradient()#gradient_with_multi-position_color_stops
A linear gradient like this can be passed to the same background prop as the other gradient types. This will allow all logic to create the stepped gradient to be moved from range_highlight to color_stops. The stepped prop should also then be removed from range_highlight.

Also if in future if range_highlight component is used

The reason that color_stops logic should not be part of range_highlight is that it is already being used in other components, namely EuiRange and EuiDualRange. We want to avoid further changes to this component, especially when its current props are sufficient for the new use case (i.e., background prop).

Made changes @thompsongl @flash1293.

Also @flash1293 @thompsongl where is the default grey colour stored maybe in some variable name of something. I am not able to find it.

About the behavior - all other palettes start at the first stop, except for stepped color segments. I think it should behave in the same way and use the default gray color for the area from start to first stop, just to keep consistency

Yes, I agree this is how it should work.

where is the default grey colour stored maybe in some variable name of something. I am not able to find it.

Use currentColor, which is a CSS feature that will use the current text color (which we've set to gray). See the other gradient examples for how it's used: https://github.com/elastic/eui/blob/master/src/components/color_picker/color_stops/color_stops.tsx#L429

If we provide the color as currentColor chroma shows the error.
@thompsongl

@git-anurag-hub Don't pass currentColor to chroma, it's a css thing. You have to do this in the place where you build the actual css background string in the component (src/components/color_picker/color_stops/color_stops.tsx)

We can't do that because we have to make the stepped background first before providing it to the css.
It is done in the linear gradient because we have to provide it directly to the css before providing it to the anything else.

@thompsongl One thing I can do is provide the hex value of the grey color you are using but I don't think it is the great idea/solution.

@git-anurag-hub Then you have to refactor your code - the helper function in util shouldn't care about the currentColor thing in the beginning, it has to be added afterwards before building the css gradient

We can't do this because we are using chromajs to make the steps and we need to provide the first color (i.e. grey). @flash1293 @thompsongl

@git-anurag-hub This is how it should work:

• the util helper using chroma calculates the colors without knowing anything about the first grey color (like it works now)
• Then, in the place where the color segments calculated by chroma are turned into a css background, the area from the start of the range to the first stop is replaced by currentColor. Here: https://github.com/elastic/eui/pull/4613/files#diff-9e5f99e28d91e63d2d521b34d71b38cefe346447b58053a087d6950957c422deR455

It might be just because you didn't push your current state, but you resolved some comments without changing anything.

@flash1293 is exactly right. You can think of the gray color before the first stop as an 'empty' space; it has not been configured with a color by the user. For that reason, currentColor should not be included as one of the 10 (by default) color steps in the gradient.
So if the first color stop is 'red' at '20', currentColor occupies the space from 0-20 and does not need to be included in the chroma function to create the stepped color gradient from 20-100.

Let me know if this makes sense, and thanks for sticking with us as we provide feedback; your help is much appreciated

Np @flash1293 @thompsongl Can please review pr once again as I made the changes asked for.
It will be great if you can please review this today so that I can work on something else tomorrow. :)

Thanks, @flash1293

This is looking good so far! A couple things I noticed while clicking around the demo:

It's possible to get into odd configurations where a colors appears before the drag handle of the previous color (e.g., pink before the yellow handle).

Low step counts have the potential to misrepresent the color distribution. Also, should we add a rule to force the step count to be at least equal to the number of drag handles (i.e., minimum of 3 in the above screenshot)?

Given your explanation that the stepped gradient is "actually just a quantization postprocessing of the smoothly interpolated palette", both of these seem to be acceptable/expected, but what to get your thoughts.

Regarding the onChange value, no information is returned to the consumer about the stepped gradient. Like the other gradient types, you simply get the drag handle values:

[
  {stop: 20, color: "#54B399"},
  {stop: 50, color: "#D36086"},
  {stop: 64, color: "#9170B8"}
]

As a consumer, do you need access to the values and start/stop locations of each gradient step?

Thanks for the check @thompsongl

It's possible to get into odd configurations where a colors appears before the drag handle of the previous color (e.g., pink before the yellow handle).

Yes, this is due to the way the palette is quantized - it's not taking the center of each segments for the color. The first segment is colored by the very first step, the last segment by the very last step, and the places to use color from in between are evenly distributed. This only matches with the center of the segment for the middle segment. This shows from where on the spectrum the individual segments get their color:

I thought back and forth on this - another thing we could do is this:

But it has other weird edge cases - now there won't be a segment colored exactly like the beginning and end colors, but always a blend - in the example above the first segment would be a blend of green and yellow, the second segment would be a green/yellow blend with less green, and the last one would be perfect yellow with no red at all - which is also weird IMHO.

I thought we can start with this and if people complain we can introduce more options - as you mentioned this is an edge case anyway.

Low step counts have the potential to misrepresent the color distribution. Also, should we add a rule to force the step count to be at least equal to the number of drag handles (i.e., minimum of 3 in the above screenshot)?

Good point, I think we can enforce this on the consumers side, but I would keep the component itself unbiased.

As a consumer, do you need access to the values and start/stop locations of each gradient step?

No, that's fine as they will be evenly distributed. As we will have some logic for this on the consumer side anyway, I think the current abstraction level is fine.

I thought we can start with this and if people complain we can introduce more options - as you mentioned this is an edge case anyway.

This makes sense to me. The core API is settled, especially if we don't enforce restrictions on step count. Any future changes would likely be visual in nature or add new options. I'll take another review of the code.

jenkins test this

Preview documentation changes for this PR: https://eui.elastic.co/pr_4613/

Snapshot test failure:

09:37:16             <div
09:37:16               class="euiRangeHighlight__progress"
09:37:16     -         style="background:linear-gradient(to right, currentColor 0%, #ff0000 0% 3.5%, #d72800 3.5% 7%, #b04f00 7% 10.5%, #887700 10.5% 14%, #609f00 14% 17.5%, #39c600 17.5% 21%, #11ee00 21% 24.5%, #00c639 24.5% 28%, #00639c 28% 31.5%, #0000ff 31.5% 35%);margin-left:0%;width:100%"
09:37:16     +         style="background:linear-gradient(to right, currentColor undefined%, #ff0000 NaN% NaN%, #d72800 NaN% NaN%, #b04f00 NaN% NaN%, #887700 NaN% NaN%, #609f00 NaN% NaN%, #39c600 NaN% NaN%, #11ee00 NaN% NaN%, #00c639 NaN% NaN%, #00639c NaN% NaN%, #0000ff NaN% NaN%);margin-left:0%;width:100%"
09:37:16             />

Jenkins, test this

@thompsongl Fixed the failing test. As I rely on the positions array now which requires a dom measurement, I changed how the test works.

Preview documentation changes for this PR: https://eui.elastic.co/pr_4613/

Jenkins, test this

Preview documentation changes for this PR: https://eui.elastic.co/pr_4613/

Jenkins, test this

Thanks @miukimiu !

Preview documentation changes for this PR: https://eui.elastic.co/pr_4613/

Jenkins, test this

Preview documentation changes for this PR: https://eui.elastic.co/pr_4613/